Skip to content

[QC-682] override stop() but do not register a callback#922

Merged
Barthelemy merged 2 commits into
AliceO2Group:masterfrom
Barthelemy:task-stop-fix
Oct 21, 2021
Merged

[QC-682] override stop() but do not register a callback#922
Barthelemy merged 2 commits into
AliceO2Group:masterfrom
Barthelemy:task-stop-fix

Conversation

@Barthelemy
Copy link
Copy Markdown
Collaborator

@Barthelemy Barthelemy commented Oct 21, 2021

No description provided.

@Barthelemy Barthelemy requested a review from knopers8 as a code owner October 21, 2021 12:21
@Barthelemy
Copy link
Copy Markdown
Collaborator Author

Barthelemy commented Oct 21, 2021

@ktf what do you think ?

@Barthelemy
Copy link
Copy Markdown
Collaborator Author

Barthelemy commented Oct 21, 2021

actually it does not work if I don't keep the callback assignment in the QC

@Barthelemy Barthelemy merged commit bd7ae21 into AliceO2Group:master Oct 21, 2021
@Barthelemy Barthelemy deleted the task-stop-fix branch October 21, 2021 14:02
@aphecetche
Copy link
Copy Markdown
Contributor

aphecetche commented Oct 23, 2021

@Barthelemy don't know if it's only on my setup, but I get compilation errors :

/Users/laurent/alice/spack/laurent/qc/QualityControl/Framework/include/QualityControl/CheckRunner.h:202:15: error: only virtual member functions can be marked 'override'
  void stop() override;

clang++ --version
Apple clang version 13.0.0 (clang-1300.0.29.3)
Target: x86_64-apple-darwin20.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

@Barthelemy
Copy link
Copy Markdown
Collaborator Author

Barthelemy commented Oct 25, 2021

@aphecetche ok, I did not see this on my mac. Trying again.

knopers8
knopers8 reviewed Oct 25, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@knopers8 knopers8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, so there is a stop method that we can override, but it does not have any effect on its own? This could be fixed in DPL perhaps...

@Barthelemy
Copy link
Copy Markdown
Collaborator Author

Barthelemy commented Oct 25, 2021

@aphecetche On my mac I don't get that. Are you sure that O2 is up to date ?
stop is virtual in O2 and thus we must mark it as override.

Barthelemy added a commit that referenced this pull request Oct 25, 2021
@Barthelemy
[QC-682] override stop() but do not register a callback (#922)
3943abc 
* [QC-682] override stop() but do not register a callback

* Put back the callback
@aphecetche
Copy link
Copy Markdown
Contributor

aphecetche commented Oct 25, 2021

@Barthelemy ahem, no, I was not using O2 dev but nightly-20211020. Sorry for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@knopers8 knopers8 knopers8 left review comments

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Barthelemy @aphecetche @knopers8